Skip to content

London | 26-ITP-Sept | Hugh Mills | Sprint 2 | practice tdd - #1697

Open
HM-127BTY wants to merge 14 commits into
CodeYourFuture:mainfrom
HM-127BTY:coursework/sprint-2-practice-tdd
Open

HM-127BTY wants to merge 14 commits into
CodeYourFuture:mainfrom
HM-127BTY:coursework/sprint-2-practice-tdd

Conversation

@HM-127BTY

Copy link
Copy Markdown

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Task code

CYF-1060

Changelist

Completed the tasks by doing tdd to the best of my ability.

@HM-127BTY HM-127BTY added 📅 Sprint 2 Assigned during Sprint 2 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 13, 2026
@hackertainment hackertainment added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 18, 2026

@hackertainment hackertainment left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your coursework. Please keep it up.

Comment thread practice-tdd/count.js

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your code is correct. Just wondering what is the advantage of using this approach over other implementations? Thank you.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course from what I understand, its when its more test driven it helps you build the code as your making tests for it, it helps keep a better flow and streamlined work on the code while addressing the tests at the same time. If you just build the code itself without tests in this way it can lead to unseen issues that end up rebuild large chunks of the code that can result in making more bugs and issues.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry maybe my original question was not clear. I didn't mean the advantages of TDD, I was wondering how would you come up with the following approach with the given test cases:

  • charMatch = str.match(new RegExp(char, "g"))
  • charMatch === null
  • return charMatch.length

Especially what is the advantage of such implementation but not other programming logics? Thank you for your claification.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was when I was building up the code, I looked for something to use to count the "char" but i cant recall which i used but was only counting the first letter so when I searched for options on MDN i found .match() and used that one.

I found out about RegExp on the same page as .match() and saw it was used to help with finding the pattern of what i was searching for.

I still need to look into it more to understand it better to be honest.

If i think of it, this takes all the occurrences of g out of the string
For the null one I ran into an error that was showing null instead of 0 at the time so i think I put in an if to catch it out to show 0.

After finding those when I was stumped I couldn't think of any other logics I could have used at the time. I think i did it diffrently before so will look it up after this.

@hackertainment hackertainment Oct 22, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, although the explanations are not quite accurate, at least I understand your thinking. Even this is not the best implementation, I would let you pass this one because the program runs correctly. Maybe try to book a mentored coding session with volunteer to try another way to solve the same exercise. Thank you.

p.s.: the g in new RegExp(char, "g") means "global search" but not searching for character "g".

Comment on lines 13 to 18
test("should count multiple occurrences of a character", () => {
const str = "aaaaa";
const char = "a";
const count = countChar(str, char);
expect(count).toEqual(5);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a special case of str where it is all the char to count. Would it still count correctly if the char just exists in some part of the str?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I forgot to do a test for that and have now put one in for that test of a random letters in a string.

Comment on lines +26 to +31
test("should count 0 with no occurrences of a character", () => {
const str = "aaaaa";
const char = "b";
const count = countChar(str, char);
expect(count).toEqual(0);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a special str that would give 0 anyway. Please add that test case as well. Thank you.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that when str has nothing in it? I have added a test for this aswell as some others that have just thought of.

Comment on lines +3 to +8
if (11 <= Number(numString.slice(-2)) && 19 >= Number(numString.slice(-2))) {
return numString + "th";
} else if (
-11 >= Number(numString.slice(-2)) &&
-19 <= Number(numString.slice(-2))
) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please be aware of the consistency of code formatting - while the first if using one line for the conditions but the second if using multiple lines for the conditions. I personally would prefer one line which may usually be easier to read, unless explicitly specified by coding standard in a business environment.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am aware of that, this is the doing of the auto format that we had to put into vscode. I tried to keep it like line 3 and it just automatically does that format, when I saw it doing that I tried to make it then be like lines 6 and 7 but then it forces it back to how it was at 3. Not sure how to stop it doing that when I hit tell it to save.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for letting me know. I will discuss with cirriculum team to improve the auto format file. All good for now.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your test cases are correct. Just for you to think a little bit more, besides const str = "hello", what special string should also be added to each of the test cases?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah i think I understand, ones where it has a space or symbols, even an empty one. Added those tests.

@hackertainment hackertainment Oct 22, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sometimes space or symbols can be boundary cases. In the context of this program, only empty and non-empty string matters. Empty string with any positive or zero count should still be empty. However, empty string with negative count should still throw error. Some trainees also return empty string for negative count which would not be quite accurate "for the context of this question". This is why we need to test empty string "" together with "hello"in each of your original test suites - not adding empty string as a separate test suite. Since your program is correct, I will let you pass this time (even testing of empty string for count = -3 is missing) but understand the design process should be your focus. You may also book a mentored coding session with a volunteer to learn more about test cases design. Thank you.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah didn't think of the negatives there, need to remeber those. I see what you mean, I'll look into the mentored coding session I got coming up to ask about it but will look into it aswell.

@hackertainment hackertainment added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Oct 19, 2026
@HM-127BTY HM-127BTY added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 21, 2026
@hackertainment

Copy link
Copy Markdown

Just remain this new comment #1697 (comment) , thank you.

@hackertainment hackertainment removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 22, 2026
@HM-127BTY HM-127BTY added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 22, 2026
@hackertainment

Copy link
Copy Markdown

Congratulation, you have completed this PR successfully.

@hackertainment hackertainment added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Reviewed Volunteer to add when completing a review with trainee action still to take. labels Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed. 📅 Sprint 2 Assigned during Sprint 2 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants